Connectors: Replace plugin.slug with plugin.file#11400
Connectors: Replace plugin.slug with plugin.file#11400jorgefilipecosta wants to merge 7 commits intoWordPress:trunkfrom
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
mukeshpanchal27
left a comment
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Thanks for the PR!
Have you consider single file plugin (hello.php)?
This should work, too. Worth including in the PHPDoc for completeness. |
|
@jorgefilipecosta, to test it in the browser we need to land Gutenberg changes and update the hash to have JS changes compatible? |
Exactly or we can test this together with Gutenberg plugin activated. |
There was a problem hiding this comment.
The existing test test_register_includes_plugin_data in tests/phpunit/tests/connectors/wpConnectorRegistry.php:297 still uses 'slug' => 'my-plugin' and passes because the registry stores whatever keys are provided in the plugin array without validation. This should be updated to use 'file' to match the new contract:
$args['plugin'] = array( 'file' => 'my-plugin/my-plugin.php' );$this->assertSame( array( 'file' => 'my-plugin/my-plugin.php' ), $result['plugin'] );This also flags that the registry might benefit from cherry-picking known properties from the plugin array (i.e., only keeping file) rather than storing whatever is passed in. That way invalid keys would be silently dropped and the stored data would always match the documented shape. It still works properly because we do that sanitization when passing data to the client.
gziolo
left a comment
gziolo
left a comment
There was a problem hiding this comment.
One small tweak to ensure OS compatibility and this is good to go. Thank you for addressing all feedback.
src/wp-includes/connectors.php
Outdated
| $is_activated = $is_installed && is_plugin_active( $plugin_file ); | ||
| if ( ! empty( $connector_data['plugin']['file'] ) ) { | ||
| $file = $connector_data['plugin']['file']; | ||
| $is_installed = file_exists( WP_PLUGIN_DIR . '/' . $file ); |
There was a problem hiding this comment.
It would be best to normalize the file name with wp_normalize_path to avoid issues with differences in how the path is constructed depending on the OS.
…ation Use the plugin's main file path (e.g. 'akismet/akismet.php') instead of just the slug. This lets the script module data function use file_exists() and is_plugin_active() directly, removing the get_plugins() slug-to-file map. The frontend derives the slug from the file path when needed.
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
jorgefilipecosta
force-pushed
the
update/connector-plugin-file
branch
from
4ca1972 to
f345615
Compare
3 participants
Summary
plugin.slugwithplugin.file(the full plugin file path, e.g.akismet/akismet.php) in connector registrationsget_plugins()slug-to-file map, usefile_exists()andis_plugin_active()directlyisInstalledto the frontend plugin dataTicket: https://core.trac.wordpress.org/ticket/65002
Test plan